Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding lenovo logger for Lenovo provider #12084

Merged
merged 1 commit into from
Oct 21, 2016

Conversation

blomquisg
Copy link
Member

Adding $lenovo_log for the Lenovo provider logger.

@blomquisg
Copy link
Member Author

As an aside, we need to figure out a way for providers to register their logger with the core app. /cc @durandom

@miq-bot
Copy link
Member

miq-bot commented Oct 20, 2016

Checked commit blomquisg@a64af1c with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
1 file checked, 1 offense detected

lib/vmdb/loggers.rb

@juliancheal
Copy link
Member

❗️ - Line 37, Col 34 - Style/GlobalVars - Do not introduce global variables.

I thought we did logging in a different way than with the $ now to appease the Rubocops?

@juliancheal
Copy link
Member

LGTM 👍

@Fryguy
Copy link
Member

Fryguy commented Oct 20, 2016

@blomquisg I had ideas about not creating separate $loggers at all. Basically, each plugin could be "given" a dedicated logger object by the server. Then the code within the plugin would call ManageIQ::Providers::Whoever.logger Then the provider is responsible for using that method, and the server is responsible for populating it on boot. Options for the logger would be within the Settings namespace for that provider. (more or less 😉 )

@Fryguy
Copy link
Member

Fryguy commented Oct 20, 2016

^^ This would of course depend on the Settings pluggability...I have to find time to review @durandom's stuff :)

@juliancheal
Copy link
Member

@Fryguy Sounds like a great idea.

@blomquisg
Copy link
Member Author

@juliancheal said:

I thought we did logging in a different way than with the $ now to appease the Rubocops?

None that I know of ...

@blomquisg
Copy link
Member Author

@Fryguy said:

I had ideas about not creating separate $loggers ...

heh, I was gonna respond about Settings ... but you beat me

@Fryguy Fryguy merged commit 1489fe7 into ManageIQ:master Oct 21, 2016
@Fryguy Fryguy added this to the Sprint 48 Ending Oct 24, 2016 milestone Oct 21, 2016
@blomquisg blomquisg deleted the add-lenovo-logger branch November 28, 2017 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants